sysroot: Load bootloader configs via boot_fd

This was a general principle cleanup, preparation
for handling VFAT for /boot for systemd-boot/BLS
support.

However I ran into an ugly corner case in our
unit tests that pointed at a sysroot without a
boot directory. The previous logic handled
ENOENT for boot/loader but not /boot.
Continue to cope with that degenerate situation.

Signed-off-by: Colin Walters <walters@verbum.org>

diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
index 5d356d4dbbe372f659b50ffcfa7e1baea01edbfc..49168d4f170395aa313cf3cc0ba1dd180c7cd784 100644 (file)
--- a/src/libostree/ostree-sysroot-deploy.c
+++ b/src/libostree/ostree-sysroot-deploy.c
@@ -2826,6 +2826,8 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self, GPtrArray *n
 
   if (!_ostree_sysroot_ensure_writable (self, error))
     return FALSE;
+  if (!_ostree_sysroot_ensure_boot_fd (self, error))
+    return FALSE;
 
   const bool skip_early_prune = (self->opt_flags & OSTREE_SYSROOT_GLOBAL_OPT_NO_EARLY_PRUNE) > 0;
   if (!skip_early_prune && !opts->disable_auto_early_prune

diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h
index 3dd6939b4c8518d68a39befce25d7cc31c68de7f..ac5d271c516e460c94e72c1860aaf49e89c21615 100644 (file)
--- a/src/libostree/ostree-sysroot-private.h
+++ b/src/libostree/ostree-sysroot-private.h
@@ -64,8 +64,13 @@ struct OstreeSysroot
   GObject parent;
 
   GFile *path;
+  // File descriptor for the sysroot. Only valid after `ostree_sysroot_ensure_initialized()`
+  // has been invoked (directly by a calling program, or transitively from another public API).
   int sysroot_fd;
+  // File descriptor for the boot partition. Should be initialized on demand internally
+  // by a public API eventually invoking `_ostree_sysroot_ensure_boot_fd()`.
   int boot_fd;
+  // Lock for this sysroot.
   GLnxLockFile lock;
 
   OstreeSysrootLoadState loadstate;

diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
index dab70486de9f7987ceffd82c3070801f655c45e4..3e237c398a563c9679a1b1b720cb1a25449db5bb 100644 (file)
--- a/src/libostree/ostree-sysroot.c
+++ b/src/libostree/ostree-sysroot.c
@@ -338,9 +338,38 @@ ensure_sysroot_fd (OstreeSysroot *self, GError **error)
   return TRUE;
 }
 
+/* Require that both self->sysroot_fd is set.
+ * If the sysroot has a boot/ subdirectory, it will be loaded.
+ * If not, self->boot_fd will remain -1.
+ *
+ * This API is only for backwards compatibility with effectively broken
+ * situations where we're pointed at a sysroot that doesn't have /boot.
+ */
+static gboolean
+_ostree_sysroot_maybe_load_boot_fd (OstreeSysroot *self, GError **error)
+{
+  if (!ensure_sysroot_fd (self, error))
+    return FALSE;
+  if (self->boot_fd == -1)
+    {
+      int fd = glnx_opendirat_with_errno (self->sysroot_fd, "boot", TRUE);
+      if (fd < 0)
+        {
+          if (errno != ENOENT)
+            return glnx_throw_errno_prefix (error, "Opening boot/");
+        }
+      else
+        self->boot_fd = fd;
+    }
+  return TRUE;
+}
+
+/* Require that both self->sysroot_fd and self->boot_fd are loaded */
 gboolean
 _ostree_sysroot_ensure_boot_fd (OstreeSysroot *self, GError **error)
 {
+  if (!ensure_sysroot_fd (self, error))
+    return FALSE;
   if (self->boot_fd == -1)
     {
       if (!glnx_opendirat (self->sysroot_fd, "boot", TRUE, &self->boot_fd, error))
@@ -607,18 +636,28 @@ _ostree_sysroot_read_boot_loader_configs (OstreeSysroot *self, int bootversion,
                                           GPtrArray **out_loader_configs, GCancellable *cancellable,
                                           GError **error)
 {
-  if (!ensure_sysroot_fd (self, error))
-    return FALSE;
-
   g_autoptr (GPtrArray) ret_loader_configs
       = g_ptr_array_new_with_free_func ((GDestroyNotify)g_object_unref);
 
-  g_autofree char *entries_path = g_strdup_printf ("boot/loader.%d/entries", bootversion);
+  // In our unit tests we have some cases where we do
+  // ostree --sysroot=/path/to/deployment/root remote add
+  // without a boot directory at all. This is a broken situation,
+  // but we attempt to cope.
+  if (!_ostree_sysroot_maybe_load_boot_fd (self, error))
+    return FALSE;
+  if (self->boot_fd == -1)
+    {
+      g_debug ("Deployment is missing boot directory");
+      *out_loader_configs = g_steal_pointer (&ret_loader_configs);
+      return TRUE;
+    }
+
+  g_autofree char *entries_path = g_strdup_printf ("loader.%d/entries", bootversion);
   gboolean entries_exists;
   g_auto (GLnxDirFdIterator) dfd_iter = {
     0,
   };
-  if (!ot_dfd_iter_init_allow_noent (self->sysroot_fd, entries_path, &dfd_iter, &entries_exists,
+  if (!ot_dfd_iter_init_allow_noent (self->boot_fd, entries_path, &dfd_iter, &entries_exists,
                                      error))
     return FALSE;
   if (!entries_exists)

diff --git a/src/ostree/ot-admin-builtin-finalize-staged.c b/src/ostree/ot-admin-builtin-finalize-staged.c
index 2ae1674da86c48035b9523f5e04680a01033b87b..1e5c2df115f822a633340cce57d1f51d19130515 100644 (file)
--- a/src/ostree/ot-admin-builtin-finalize-staged.c
+++ b/src/ostree/ot-admin-builtin-finalize-staged.c
@@ -71,20 +71,10 @@ ot_admin_builtin_finalize_staged (int argc, char **argv, OstreeCommandInvocation
               cancellable, error))
         return FALSE;
 
-      /* In case it's an automount, open /boot so that the automount doesn't
-       * expire until before this process exits. If it did expire and got
-       * unmounted, the service would be stopped and the deployment would be
-       * finalized earlier than expected.
-       */
-      int sysroot_fd = ostree_sysroot_get_fd (sysroot);
-      glnx_autofd int boot_fd = -1;
-      g_debug ("Opening /boot directory");
-      if (!glnx_opendirat (sysroot_fd, "boot", TRUE, &boot_fd, error))
-        return FALSE;
-
-      /* We want to keep /boot open until the deployment is finalized during
+      /* By default the sysroot now holds a file descriptor for /boot open.
+       * We want that open until the deployment is finalized during
        * system shutdown, so block until we get SIGTERM which systemd will send
-       * when the unit is stopped.
+       * when the unit is stopped, then we'll exit and release it.
        */
       pause ();
     }

diff --git a/tests/kolainst/destructive/boot-automount.sh b/tests/kolainst/destructive/boot-automount.sh
index 17e44acaed6cd178914b7608c480bdb833851b07..29c72efd6dffee7bcafb14899235a7dff5c35db2 100755 (executable)
--- a/tests/kolainst/destructive/boot-automount.sh
+++ b/tests/kolainst/destructive/boot-automount.sh
@@ -21,6 +21,9 @@ EOF
     systemctl daemon-reload
     systemctl enable boot.automount
 
+    # Stop this as it may also be holding /boot open now
+    systemctl stop rpm-ostreed.service
+
     # Unmount /boot, start the automount unit, and ensure the units are
     # in the correct states.
     umount /boot